Skip to content

Add field parking:$side and parking:$side:orientation#674

Closed
tordans wants to merge 11 commits intoopenstreetmap:mainfrom
tordans:parking_centerline
Closed

Add field parking:$side and parking:$side:orientation#674
tordans wants to merge 11 commits intoopenstreetmap:mainfrom
tordans:parking_centerline

Conversation

@tordans
Copy link
Copy Markdown
Collaborator

@tordans tordans commented Dec 8, 2022

Solves #675


Update 2022-12-29

I created #722 (and #723) as minimal versions of this PR. More about that over there.

I consider this PR a collection of TODOs and know how. Once the issues are solved, we should create separate PRs.

Update 2022-12-28

This part is a must have:

This part is a nice to have and IMO can be follow up tickets:

  • The :both case needs to be solved, see Cycleway:both is ignored iD#9212 (comment) for some more info about it
    • For this PR, supporting :left, :right, :both would be sufficient.
    • For the linked issue there is also the :<null> and :<null>+oneway=yes case, which makes it quite a bit more complex, so maybe we can split this in two issues?
    • The cycleway field will need a review once this is solved ede72b7.
  • The prerequisiteTag needs to be figured out
    • See comment below for more.
    • See also c4e218a
  • We should to add "descriptions" to the fields, see d9fc26a
    • @SupaplexOSM can you help with this part?
    • The description shows as a tooltip when hovering the dropdown option
    Bildschirm­foto 2022-12-28 um 14 13 15

@tordans tordans force-pushed the parking_centerline branch from 0aefeb2 to bd84550 Compare December 8, 2022 15:48
"perpendicular": "Meets the Street at a Straight Angle"
}
},
"autoSuggestions": false,

This comment was marked as outdated.

@tyrasd tyrasd added the new-field create a new field (see add-field for cases where field from presets is added to new entries) label Dec 8, 2022
@tyrasd
Copy link
Copy Markdown
Member

tyrasd commented Dec 8, 2022

How to handle $side within a present field?

In principle, this would be a good fit for the cycleway field type (see the Special section in the schema -builder docs).

image

The field json would look like this:
{
  "keys": [
      "parking:both",
      "parking:left",
      "parking:right"
  ],
  "type": "<<cycleway>>",
  "label": "Street Parking Type",
  "strings": {
      "types": {
          "parking:left": "Left side",
          "parking:right": "Right side"
      },
      "options": {
          "lane": "Roadside Lane",
          "street_side": "Street-Side",
          "on_kerb": "On Kerb",
          "half_on_kerb": "Half On Kerb",
          "shoulder": "Shoulder",
          "no": "No",
          "separate": "Parking mapped separately",
          "yes": "Yes (unspecified)"
      }
  },
  "autoSuggestions": false,
  "customValues": false
}

However, the implementation is pretty much hardcoded for the literal cycleway:* key on iD's side. It wouldn't be too hard to generalize this into a reusable ("type": "directional") field type.

For this type of field, it would make sense if the prerequisiteTag feature would also look at the corresponding $side subtags: e.g., if parking:left is no or empty and parking:right is set, only the parking:right:orientation input field should be enabled. I'm however not jet sure how to best model it in the schema. Probably it would make the most sense to be able to reference another field as a prerequisite in such cases:

"prerequsiteTag": {
    "field": "parking/side"
}

@tordans
Copy link
Copy Markdown
Collaborator Author

tordans commented Dec 28, 2022

@tyrasd thanks for the input in your comment above and the update on iD. I updated this PR-description with a list of things that need to happen next from my point of view.

Would you be up to merging this with "must have" solved? And handle the other issues as follow ups?


On the topic of prerequisiteTag:

This needs an update to the schema-builder. But how to specify best, what we want? IMO, we want…

The field should appear if …

  • any of the keys parking:left,parking:right,parking:both is present
  • AND the values are not separate, no

We need the "not" part, since we don't users to provide an :orientation when so side was given.

This has two issues:

  • We need to check combinations of key+value, right? Only if all given keys have a value of separate, no, the field should be hidden. Otherwise …

  • If parking:right=no|separate, then parking:right:orientation should not be visible. Which would be another feature of the directionalCombo. Either the field should be deactivated; or a validation should warn users once they added data that this is (possible/doable but) wrong.

    Maybe we could work around this, by only showing :orientation directionalCombo fields if :right, :left is given, and a regular dropdown if :both is given.


Update 2022-12-29: In 47f61c8#diff-ffcc47af10b2693db223dcfd1da1260b54c5524e4af34ba8d4776ee24cb041eaR21-R79 I added a bit of code to experiment with options to solve the requirement to extend the prerequisiteTag.


Let me know what you think.

…onalCombo)

Those commits also hold some todo comments on the `prerequisiteTag`.
…mbo)

This way the left/right is exposed to the users. However, there is no convenient way to switch between those types, other than changing the raw tags. Ping openstreetmap/iD#9212 (comment)
The cycleway use it, so we will likely want it here as well, right?
@tordans
Copy link
Copy Markdown
Collaborator Author

tordans commented Jan 20, 2023

I will close this now in favor of #744.

It will be a while until the prerequisiteTag is extended enough to add more fancyness to this feature. And likely openstreetmap/iD#9476 will be a first step by manually handling those cases in iD.

@tordans tordans closed this Jan 20, 2023
@tordans tordans deleted the parking_centerline branch January 20, 2023 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-field create a new field (see add-field for cases where field from presets is added to new entries)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants